fix(templates): surface feature files/ directories the manifest ignores - #3796
Conversation
|
Warning Review limit reached
Next review available in: 30 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76ed86f4e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kwakayama
left a comment
kwakayama
left a comment
There was a problem hiding this comment.
Finding
- HIGH — Restore
ChatSidebar.Item.Titleor provide an explicit breaking-change migration.ChatSidebarItemCompoundnow exposes onlyMenu,Rename, andDeleteatsrc/react/components/chat/chat/components/sidebar.tsx:601, removing the previously publicItem.Titleruntime API and its props type. Existing consumers using<ChatSidebar.Item.Title />will fail typechecking, while JavaScript consumers hit an undefined React component at runtime. The PR also deletes the compatibility tests and rewrites the RFC to claim the API never existed (docs/rfcs/29-chat-api-shape/components/chat-sidebar.md:7), despite this being unrelated to feature-file detection. Restore the API or isolate this as a documented major-version migration with replacement guidance and retained regression coverage.
| Rubric | Score |
|---|---|
| Correctness | 20/40 |
| Tests | 10/20 |
| Reliability/Security | 15/15 |
| Maintainability | 11/15 |
| Scope/Docs | 6/10 |
| Total | 62/100 |
Review-Gate:
Reviewer: Codex
Reviewed-SHA: e1cae60
Score: 62/100
Actionable-Findings: 1
Verdict: REQUEST_CHANGES
e1cae60 to
3ab452d
Compare
|
Correction: my previous review note referenced files that are not present in this PR GitHub diff. It came from comparing against an unrelated local tree and is withdrawn. This PR changes only deno.json, scripts/build/feature-files-detection.test.ts, and scripts/build/generate-templates-manifest.ts. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Closes #3786.
The answer: neither reading, and not specific to mdx
#3786 asked whether
templates/features/mdx/files/is dead weight or a loader bug. The mechanism was never wired, and it affects all six features:FeatureConfigintemplates/types.tshas nofilesfield, nofeature.jsondeclares one, and the loader never reads the directory. The issue's premise — "mdx's feature.json has no files key" — is true of every feature and was never how files were meant to be declared.The generator now reports this on every run instead of passing over it silently. That is the acceptance item that holds under either reading.
Why this does NOT activate the files
Activating them was implemented, reviewed, and discarded — it produces regressions that were proven, not suspected:
aicollides with the template it is meant to extend.templates/features/ai/files/shipsagents/assistant.tsandapp/api/ag-ui/route.ts; I verified against the manifest thatai-agentalready owns both paths. Activating means--template ai-agent --features aisilently overwrites template files with versions that have never been exercised.--features authand--features redisscaffold projects that faildeno linton first run.Switching on 29 never-executed files across six features, half of them broken, is a far larger change than "are these three mdx files dead?". Each feature needs fixing and covering; that is its own piece of work, filed separately.
Reported rather than thrown for the same reason — making it fatal breaks the build until all six are resolved, and that is not a manifest-generator decision.
Tests
Five. Four run against temp fixtures so they do not depend on the real tree. The fifth asserts only that the condition exists today, not which features — so when #3786's successor lands it flips to empty and fails loudly, which is the reminder to delete it.
Note
templates/manifest.generated.tsreads as stale onorigin/mainbefore this branch too — regenerating produces a different gzip blob with identical content (48 templates, 465 files). This PR deliberately does not commit that churn, since aconsole.warncannot alter manifest content.